Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

Smiley fix (part 1 from issue #83) #95

Closed
wants to merge 2 commits into from
Closed

Conversation

looph
Copy link

@looph looph commented Feb 3, 2014

Fixes first point in issue #83:

  • Move the default smiley pack into a resource directory and treat it just as if it was located in a file system, instead of hardcoding the config for the smiley pack in cpp. That would also allow the default slimey pack to be moved out of .exe if needed without any trouble.

Solution:

  • /resources/icon/theme.th file has been created. Contains previous default smiley pack info.
  • Removed default smiley pack from smileypack.cpp.
  • Added extension type .th to identify theme files.
  • scans two directories for theme files: ~/.config/ and ../resources/icon/

@nurupo
Copy link
Owner

nurupo commented Feb 3, 2014

I think you just broke smiley pack compatibility with pidgin smiley packs.

  • There is no need for theme file extensions.
  • There should no be any paths in the theme file, just file names.

../resources/icon/, there will be no such path when the binary will be shipped.

All smileys from executable's resources should be removed, they will be placed into QStandardPaths::writableLocation(QStandardPaths::ConfigLocation) + '/' + "smileys" in their own directory called default by installer or something.

Also, make sure your commits have valid emails, not per@ubuntu.(none).

@looph
Copy link
Author

looph commented Feb 4, 2014

thanx for the feedback. Sorry, I missed the email.. (my first commit to github)

Ok, so we should follow the pidgin format of smiley pack?: https://developer.pidgin.im/wiki/SmileyThemes

then shall we support:

  • ! in front of hidden smileys?
  • packs with multiple themes? i.e. read smileys under [TOX], or (if TOX is not present) [default]?.

I noticed in pidgin they have smiley packs that are included in the executable.

@looph looph closed this Feb 4, 2014
@looph looph deleted the smiley-fix branch February 4, 2014 12:16
@nurupo
Copy link
Owner

nurupo commented Feb 5, 2014

! in front of hidden smileys?

I guess we will not be fully compatible then, heh.
I personally don't see any point in hidden smilies, so we could parse them as regular smilies.
@Schlumpf, what do you think?

The extra keys, like website and email are not required, so besides '!', which we would just ignore, it's pretty compatible.

packs with multiple themes? i.e. read smileys under [TOX], or (if TOX is not present) [default]?

Yes, we would parse [tox|default|theme|smileys] (currently looking for just theme|smileys]).

@Schlumpf
Copy link
Contributor

Schlumpf commented Feb 5, 2014

I agree, we should ignore the ! and use these smileys as normal ones. We have nothing to hide ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants